Minor issue fixes#13627
Conversation
WalkthroughAdds importedCase and country fields to epidemiological data across API, backend entity/facade, UI form, i18n captions, and SQL schema. Updates DTO annotations, ORM mappings, facade mapping, UI visibility and inputs, and resources. Extends schema with new columns and adds related exposure columns and symptom type adjustments. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant UI as EpiDataForm (UI)
participant F as EpiDataFacadeEjb
participant CS as CountryService
participant DB as Database
U->>UI: Set Imported Case / Country
UI->>F: save(EpiDataDto{importedCase, countryRef})
activate F
F->>CS: findByUuid(countryRef.uuid)
CS-->>F: Country entity
F->>DB: Persist EpiData{importedCase, country_id}
DB-->>F: OK
F-->>UI: Saved DTO
deactivate F
Note over UI,F: On load/edit
UI->>F: getByCase(...)
F->>DB: Fetch EpiData + Country
DB-->>F: Entity
F-->>UI: EpiDataDto{importedCase, countryRef}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java (1)
175-179: Respect visibility checkers when toggling COUNTRY; prefer setVisibleWithCheckersWhen.Current setVisibleWhen can re-show fields disallowed by FieldVisibilityCheckers after user interaction. Use the checker-aware helper.
- FieldHelper.setVisibleWhen(getFieldGroup(), EpiDataDto.COUNTRY, EpiDataDto.IMPORTED_CASE, YesNoUnknown.YES, true); + FieldHelper.setVisibleWithCheckersWhen( + getFieldGroup(), + EpiDataDto.COUNTRY, + EpiDataDto.IMPORTED_CASE, + YesNoUnknown.YES, + getFieldVisibilityCheckers(), + true);Optionally, make COUNTRY required when IMPORTED_CASE == YES for better data quality.
Also applies to: 193-193
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
sormas-api/src/main/java/de/symeda/sormas/api/epidata/EpiDataDto.java(4 hunks)sormas-api/src/main/java/de/symeda/sormas/api/i18n/Captions.java(1 hunks)sormas-api/src/main/resources/captions.properties(1 hunks)sormas-backend/src/main/java/de/symeda/sormas/backend/epidata/EpiData.java(5 hunks)sormas-backend/src/main/java/de/symeda/sormas/backend/epidata/EpiDataFacadeEjb.java(4 hunks)sormas-backend/src/main/resources/sql/sormas_schema.sql(4 hunks)sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java (2)
sormas-api/src/main/java/de/symeda/sormas/api/FacadeProvider.java (1)
FacadeProvider(127-594)sormas-ui/src/main/java/de/symeda/sormas/ui/utils/FieldHelper.java (1)
FieldHelper(56-1270)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: android app test (26)
- GitHub Check: android app test (27)
- GitHub Check: android app test (28)
- GitHub Check: SORMAS CI
🔇 Additional comments (8)
sormas-api/src/main/resources/captions.properties (1)
1156-1157: LGTM! Caption additions are clear and well-placed.The new i18n keys for
EpiData.importedCaseandEpiData.countryare properly formatted and provide clear descriptive text that aligns with the new fields added to the epidemiological data model for GIARDIASIS disease surveillance.sormas-api/src/main/java/de/symeda/sormas/api/epidata/EpiDataDto.java (1)
35-35: LGTM! New fields are properly implemented.The additions of
importedCaseandcountryfields are well-structured:
- Appropriate imports added (line 35)
- Public constants defined for field names (lines 69-70)
- Fields correctly annotated with
@Diseases({Disease.GIARDIASIS})(lines 81-83, 133-135)- Standard getters and setters implemented (lines 280-294)
- Types are appropriate:
YesNoUnknownfor boolean-like state andCountryReferenceDtofor country referenceThe
clone()method doesn't require updates since both new fields are simple types that don't need deep cloning.Also applies to: 69-70, 81-83, 133-135, 280-294
sormas-backend/src/main/java/de/symeda/sormas/backend/epidata/EpiData.java (1)
29-30: LGTM! Entity mappings follow JPA best practices.The new fields are properly implemented:
importedCase(line 62) uses@Enumerated(EnumType.STRING)for database-friendly enum storage (lines 233-236)country(line 74) uses@ManyToOne(fetch = FetchType.LAZY)for efficient reference loading (lines 242-245)- All necessary imports added (lines 29-30, 42)
- Standard getters and setters follow convention (lines 237-240, 247-249)
The lazy loading strategy for the
countryrelationship is appropriate as it avoids unnecessary database joins when the country information is not needed.Also applies to: 42-42, 62-62, 74-74, 233-249
sormas-backend/src/main/java/de/symeda/sormas/backend/epidata/EpiDataFacadeEjb.java (3)
42-44: Country facade/service imports are correct.
63-65: EJB injection of CountryService is fine.
115-117: Null-safety for country mapping; please confirm helpers handle nulls.If source.getCountry() is null, ensure countryService.getByReferenceDto and CountryFacadeEjb.toReferenceDto return null safely to avoid NPEs. If not guaranteed, guard the calls.
Also applies to: 281-283
sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java (1)
85-85: Layout row for IMPORTED_CASE + COUNTRY — LGTM.sormas-api/src/main/java/de/symeda/sormas/api/i18n/Captions.java (1)
1569-1574: New EpiData caption keys verified
captions.properties lines 1156–1157 containEpiData.importedCaseandEpiData.country.
| @Diseases({ | ||
| Disease.MEASLES, | ||
| Disease.GIARDIASIS }) | ||
| Disease.MEASLES }) | ||
| private CaseImportedStatus caseImportedStatus; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the removal of GIARDIASIS from caseImportedStatus.
The @Diseases annotation on caseImportedStatus now only includes Disease.MEASLES, with Disease.GIARDIASIS removed. This change aligns with the introduction of the new importedCase field for GIARDIASIS (lines 81-83). However, if existing GIARDIASIS cases previously used caseImportedStatus, this could be a breaking change.
Please confirm:
- Are there existing GIARDIASIS cases in production that use
caseImportedStatus? - If yes, is there a data migration strategy to move this data to the new
importedCasefield? - Should the database schema changes include a migration script to handle this transition?
You can verify existing usage with:
🏁 Script executed:
#!/bin/bash
# Search for GIARDIASIS and caseImportedStatus usage together in the codebase
rg -n --type=java -C3 'caseImportedStatus' | grep -i giardiasis
# Check if there are any database migration scripts for this change
fd -e sql -e xml | xargs rg -l 'caseImportedStatus\|importedCase' | head -20Length of output: 96
Remove stale GIARDIASIS entry and add migration
- EpiDataDto.java (lines 77–83):
@Diseasesstill includesDisease.GIARDIASIS—remove it to match intended MEASLES-only usage. - No SQL/XML migration found for moving existing GIARDIASIS data to the new
importedCasefield—add a DB migration script to preserve and transfer that data.
🤖 Prompt for AI Agents
In sormas-api/src/main/java/de/symeda/sormas/api/epidata/EpiDataDto.java around
lines 77 to 79, remove the stale Disease.GIARDIASIS entry from the @Diseases
annotation so it only lists Disease.MEASLES, and create a DB migration (SQL or
XML Flyway/Liquibase script) that finds rows still marked with GIARDIASIS in the
old field, maps those values to the new importedCase field for the corresponding
records, and clears or deprecates the old field; ensure the migration is
idempotent, covers all affected tables/columns, includes a short descriptive
versioned filename and test/verification steps in the migration comment.
| alter table epidata add column if not exists importedCase varchar(255); | ||
| alter table epidata add column if not exists country_id bigint; |
There was a problem hiding this comment.
Don’t re-create an existing FK with IF NOT EXISTS.
epidata_country_id_fk already exists in deployed databases. Re-running this script now tries to add the same foreign key again and Postgres errors because ALTER TABLE … ADD CONSTRAINT … IF NOT EXISTS isn’t supported. Drop-and-recreate or wrap the new constraint in a migration that first checks pg_constraint; otherwise the migration blocks releases.
🤖 Prompt for AI Agents
sormas-backend/src/main/resources/sql/sormas_schema.sql around lines
14685-14686: the migration blindly recreates the epidata_country_id_fk foreign
key which already exists and Postgres does not support ALTER TABLE ... ADD
CONSTRAINT IF NOT EXISTS; change the migration so it does not attempt to add the
FK unconditionally — either write a proper migration that first checks
pg_constraint (select to see if epidata_country_id_fk exists) and only issues
the ALTER TABLE ... ADD CONSTRAINT when absent, or perform a safe
drop-and-recreate: in a transactional migration drop the existing
epidata_country_id_fk if present and then add the new constraint; ensure the
check/drop logic runs in SQL (or the project’s migration framework) so
re-applying the script won’t error and will not block releases.
KarnaiahPesula
deleted the
feature-13602-giardiasis-case-based-surveillance
branch
2 participants
Fixes #
Summary by CodeRabbit
New Features
UI
Localization